Conversation
rwood-97
left a comment
rwood-97
left a comment
There was a problem hiding this comment.
Hi Jihye,
I've added a few small comments.
Could you also have a go at updating the docs, this is the file you'd need to edit: https://github.com/maps-as-data/MapReader/blob/main/docs/source/using-mapreader/step-by-step-guide/4-classify/train.rst
The unit tests are failing at the moment but this isn't your fault - it is a dependency issue I think so I will try fix these in a separate branch.
The only one you need to fix is the check changelog test, this basically checks you've updated the changelog CHANGELOG.md as part of your PR. To fix it you just need to update the CHANGELOG.md file with your changes.
Thanks for doing this :)
| ) | ||
|
|
||
| self.labels_map = labels_map | ||
| self.huggingface = huggingface |
There was a problem hiding this comment.
I would skip setting self.huggingface since its only referenced further down in the init and instead just use the huggingface value in the if statement below
| num_labels=num_labels, | ||
| ignore_mismatched_sizes=True | ||
| ).to(self.device) | ||
| self.hf_processor = AutoImageProcessor.from_pretrained(model) |
There was a problem hiding this comment.
I think this could also be just hf_processor instead of an attribute self.hf_processor since it isn't used outside of this function
| is_inception: bool = False, | ||
| load_path: str | None = None, | ||
| force_device: bool = False, | ||
| huggingface=False, |
There was a problem hiding this comment.
can you add a type hint here
| huggingface=False, | |
| huggingface: bool = False, |
| ignore_mismatched_sizes=True | ||
| ).to(self.device) | ||
| self.hf_processor = AutoImageProcessor.from_pretrained(model) | ||
| self.input_size = getattr(self.hf_processor, "size", {"height": 224})["height"] |
There was a problem hiding this comment.
Looking here there seem to be 3 options for how size is defined.
Could you implement these, i.e.:
| self.input_size = getattr(self.hf_processor, "size", {"height": 224})["height"] | |
| size = getattr(hf_processor, "size", {}) | |
| if "height" in size and "width" in size: | |
| self.input_size = (size["height"], size["width"]) | |
| elif "shortest_edge" in size: | |
| self.input_size = (size["shortest_edge"], size["shortest_edge"]) | |
| else: | |
| self.input_size = input_size |
2 participants
Summary
This pull request adds support for Hugging Face models within the
ClassifierContainer. Previously, users had to manually load Hugging Face models and feature extractors before passing them to the container. Now, by simply passing a Hugging Face repository path and setting thehuggingface=Trueflag, the container handles the initialization automatically. (It is a part of the participation in the hut101 opportunities)Fixes #192
Describe your changes
ClassifierContainer.__init__: Added ahuggingfaceboolean flag (defaulting toFalse).transformerslibrary:AutoModelForImageClassification.from_pretrained.ignore_mismatched_sizes=Trueto allow easy fine-tuning on custom labels.self.is_inception = Falsefor HF models to bypass legacy Inception-specific logic while maintaining the existing_get_logitsworkflow.getattrto dynamically setself.input_sizefrom the processor's configuration, ensuring compatibility across different HF models.Checklist before assigning a reviewer (update as needed)
Reviewer checklist
Please add anything you want reviewers to specifically focus/comment on.